Skip to content

🐛 Start server after launching a browser and creating a build#81

Merged
wwilsman merged 2 commits into
masterfrom
ww/start-server-order
Nov 9, 2020
Merged

🐛 Start server after launching a browser and creating a build#81
wwilsman merged 2 commits into
masterfrom
ww/start-server-order

Conversation

@wwilsman

@wwilsman wwilsman commented Nov 9, 2020

Copy link
Copy Markdown
Contributor

Purpose

When using the exec:start command it downloads a browser if needed just like the exec command. However, the exec command waits for the start process to resolve directly before calling the provided command while the user/SDK is left to check this themselves when using exec:start. The healthcheck endpoint resolves when the server is listening, but the server is actually listening before a browser is downloaded and the process is actually ready.

Approach

Move the server start process to after the browser launch and build creation processes. This ensures that the server, and therefore the healthcheck endpoint, will only resolve when actually ready.

To test this, the various methods are stubbed to capture a timestamp and the resulting timestamps are compared after starting the whole process.

@wwilsman wwilsman added the 🐛 bug Something isn't working label Nov 9, 2020
@wwilsman wwilsman requested a review from Robdel12 November 9, 2020 17:59

@Robdel12 Robdel12 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁

@wwilsman wwilsman merged commit 7ebb693 into master Nov 9, 2020
@wwilsman wwilsman deleted the ww/start-server-order branch November 9, 2020 20:20
Sriram567 added a commit that referenced this pull request Jun 16, 2026
…dows CI)

The 4 self-hosted /percy/maestro-screenshot tests hardcoded POSIX paths
under `/tmp/percy-self-hosted-*`. Windows has no `/tmp/`, so on the
Windows CI runner:
  - fs.mkdirSync('/tmp/...') doesn't create the expected fixture root
  - the relay's `path.isAbsolute('/tmp/...')` is false on Windows
  - PERCY_MAESTRO_SCREENSHOT_DIR rejects, glob returns no match
  - all 4 tests 404 — caused both the original task #81 failures and
    the persistent Windows-only Test @percy/core red status on cli#2261

Fix: use `path.join(os.tmpdir(), 'percy-self-hosted-...')` so the
fixtures land in the OS's real temp directory on every platform.
Replace string-template path concatenation with `path.join(...)` to
keep separators platform-correct.

Production code is unaffected (it accepts absolute paths from the
customer regardless of platform). This is purely a test-fixture
portability fix.

Resolves task #81 (self-hosted unit-test failures on Windows / memfs+
fast-glob interaction).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants